Feat: Page Indicator (v1) #667
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Joshua-Lester3
had a problem deploying
to
BuildAndUploadImage
GitHub Actions
Failure
BenjaminMichaelis
force-pushed
the
main
branch
from
70452a1 to
23ff1d6
Compare
Joshua-Lester3
had a problem deploying
to
BuildAndUploadImage
GitHub Actions
Failure
Updates code to use `PrimaryKey` instead of `Key` property in `SiteMapping` for identifying pages. This change improves clarity and consistency by using a more descriptive name for the primary key field.
BenjaminMichaelis
force-pushed
the
dev.PageIndicator
branch
from
2d95ac3 to
47fe4d1
Compare
Modifies the FindPercentComplete method to return null when the key is null. Updates the unit test to reflect the change in behavior. Updates styling for elements related to current page display.
There was a problem hiding this comment.
Pull Request Overview
Adds a visual progress indicator to content pages by calculating and displaying the percentage of pages viewed, with supporting style and test updates.
- Introduces a new
FindPercentCompleteextension and exposespercentCompletein the layout and JS app. - Adds CSS rules for
.page-menuand.menu-progressto style the progress indicator. - Updates tests to cover basic
FindPercentCompletescenarios.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wwwroot/js/site.js | Added percentComplete ref, isContentPage computed, and exposed progress state |
| wwwroot/css/styles.css | Added .page-menu and .menu-progress styles for the progress display |
| Views/Shared/_Layout.cshtml | Passes percentComplete from server, updates layout to show percentage |
| Services/SiteMappingListExtensions.cs | Implements FindPercentComplete extension with XML docs |
| Controllers/HomeController.cs | Sets ViewBag.CurrentPageKey used by FindPercentComplete |
| Services/SiteMappingService.cs | Cleaned up stray whitespace |
| Services/ISiteMappingService.cs | Normalized namespace declaration |
| Tests/SiteMappingTests.cs | Added tests for null, whitespace, valid, and empty-list scenarios |
Comments suppressed due to low confidence (2)
EssentialCSharp.Web.Tests/SiteMappingTests.cs:138
- There’s no test for the scenario where the key isn’t present in the mappings list. Add a test to verify the behavior (e.g., returns null or "0.00").
Assert.Equal("0.00", percent);
EssentialCSharp.Web/Extensions/SiteMappingListExtensions.cs:33
- The XML comment indicates a formatted double but the method returns a string; update the
<returns>to reflect that a formatted string is returned.
/// <returns>Returns a formatted double for use as the percent complete.</returns>
Ensures that the FindPercentComplete method returns "0.00" when a key is not found in the sitemap, preventing potential incorrect calculations or errors.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added progress percent to top of page. I want to iterate on this feature with some of the following possible changes:
Fixes IntelliTect-dev/EssentialCSharp.Tooling#442
Ensure that your pull request has followed all the steps below: